Conversation
# Conflicts: # cli/common/bsb.js
cknitt
force-pushed
the
runtime-package
branch
from
893b57d to
fb158d9
Compare
cknitt
force-pushed
the
runtime-package
branch
2 times, most recently
from
9a5d72b to
4d7b90d
Compare
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
cknitt
force-pushed
the
runtime-package
branch
from
4d7b90d to
78b6bfe
Compare
cknitt
force-pushed
the
runtime-package
branch
from
12150c9 to
b9f8e3f
Compare
|
So, how would this work? I install rescript, and I was expecting a |
Yes, it's installed automatically as a dependency of the
No, it is precompiled like it was before, nothing changed there. Only the artifacts (lib/ocaml, lib/es6, lib/js) are moved from the
It's there: https://github.com/rescript-lang/rescript/blob/runtime-package/packages/%40rescript/runtime/package.json |
|
I see, this is more of a first step in a larger plan. |
|
Will not including |
Maybe the commit message was not good, the last commit just prevents them from being included in the package root in addition to |
| let files = | ||
| dirs |> StringSet.elements | ||
| |> List.map (fun name -> Files.collect name isSourceFile) | ||
| |> List.map (fun name -> Files.collect ~maxDepth:2 name isSourceFile) |
There was a problem hiding this comment.
Curious, why is a maxDepth needed after this PR?
There was a problem hiding this comment.
This part is from @cometkim.
From what I have seen, infinite recursion can occur during module resolution without it because rescript depends on @rescript/runtime, but @rescript/runtime also has rescript as a dev dependency.
Maybe we should add a comment there.
Would be nice to change the name of this folder, but I think this is outside the scope of this PR and will also most probably break some tooling. |
| if Files.exists path then Some path | ||
| else if Filename.dirname startPath = startPath then None | ||
| else resolveNodeModulePath ~startPath:(Filename.dirname startPath) name | ||
| if name = "@rescript/runtime" then |
There was a problem hiding this comment.
I don't remember exactly, but this is incomplete code. I think this is incompatible with the external-stdlib feature already.
I tried rewriting it to avoid relying on ninja's special stdlib resolution rules, but I wasn't successful. The current resolution rules still heavily rely on the node_modules directory structure and the runtime package name, which could be broken by package managers' storage design.
There was a problem hiding this comment.
I don't know much about the module resolution. What's the problem? Is it problem we handle as we should elsewhere but not in analysis/tools?
6 participants
This builds on @cometkim's work in #7483 to extract the runtime files from the main
rescriptpackage into a new package@rescript/runtime, resolving a part of #6183.Removal of
@rescript/stdshall be done in a separate PR.